Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reskin | App Layout and Header #1579

Merged
merged 18 commits into from
Apr 23, 2024

Conversation

Da-Colon
Copy link
Contributor

@Da-Colon Da-Colon commented Apr 18, 2024

  • Moves Logo to Header
  • Updates Logo to Decent Logo
  • Updates Header BG
  • Updates Search Bar Position and Icon
  • Updates Favorites and Wallet MenuButtons with Button variant, Icon and position.

Next PR

  • Clean up Wallet Menu components
  • Clean up Favorites Menu components
  • Clean up Search Bar Popup components

Issue

Works on #1560

Screenshot

image

Blurred Content:
image

@Da-Colon Da-Colon added the re-skin Temporary Label for Reskin Work label Apr 18, 2024
@Da-Colon Da-Colon self-assigned this Apr 18, 2024
- replaced `useDisplayName` with `useDAOName` to show dao name when found.
@Da-Colon Da-Colon changed the title [Part 1] App Layout and Header Reskin | App Layout and Header [Part 1] Apr 20, 2024
Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for fractal-dev ready!

Name Link
🔨 Latest commit a2c48fc
🔍 Latest deploy log https://app.netlify.com/sites/fractal-dev/deploys/66279f703481e000084a7920
😎 Deploy Preview https://deploy-preview-1579.app.dev.fractalframework.xyz
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@adamgall
Copy link
Member

adamgall commented Apr 22, 2024

The results popup, when searching for a DAO, renders inside the header component, making it get much larger than intended.

Screenshot 2024-04-22 at 5 05 18 PM

... but before I submitted this comment, I went ahead and tried to fix it. Think i got something good enough.

Screenshot 2024-04-22 at 5 04 31 PM

edit: I'm just now noticing the note in this PR

Next PR

...

  • Clean up Search Bar Popup components

edit2: aaaand you've already implemented that next set up changes in a follow up branch/pr! Undoing the hacky thing I did in favor of the better code you wrote @Da-Colon

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's merge it and keep cleaning up in future PRs

@adamgall adamgall force-pushed the reskin/1560-app-layout-0 branch from e8df5a6 to d1b3e2c Compare April 22, 2024 21:09
`Reskin` | App Layout and Header [Part 2]
Copy link
Contributor

@DarksightKellar DarksightKellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Noticed 2 suspicious colours that I flagged. If needs removal, can be cleaned up in a follow up

px={8}
pt={8}
>
<Divider color="chocolate.700" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a leftover colour that needs to be removed?

>
<DrawerOverlay />
<DrawerContent
bg="chocolate.900"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a leftover colour that needs to be removed?

@DarksightKellar DarksightKellar changed the title Reskin | App Layout and Header [Part 1] Reskin | App Layout and Header Apr 23, 2024
@mudrila mudrila merged commit 4b61448 into reskin/refactor-root-reskin Apr 23, 2024
7 checks passed
@mudrila mudrila deleted the reskin/1560-app-layout-0 branch April 23, 2024 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
re-skin Temporary Label for Reskin Work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants